feat(ui5-toolbar): implement WAI-ARIA toolbar keyboard navigation#13622
feat(ui5-toolbar): implement WAI-ARIA toolbar keyboard navigation#13622plamenivanov91 wants to merge 17 commits into
Conversation
…2 (prevent page scroll on unhandled up/down)
Refactor toolbar keyboard handling around a single toolbar-owned flow. - centralize arrow and tab navigation in Toolbar - add movement-info and roving-tabindex hooks to ToolbarItemBase - adapt grouped ToolbarItem content through shared internal target logic - restore caret-aware movement for Input and TextArea - apply forced tabindex to toolbar button/select templates - remove redundant select-owned keyboard handling - add Toolbar regressions for checkbox groups and overflow-button exit
GitHub Actions
Inactive
|
🚀 Deployed on https://pr-13622--ui5-webcomponents-preview.netlify.app |
GitHub Actions
Inactive
- Skip focusin/keydown handling when focus is inside the open overflow popover, preventing arrow-nav logic from firing inside the popover - Skip forcedTabIndex on overflowed ToolbarButton/ToolbarSelect so overflow items keep their natural tab order - Fix Tab-exit containment check to use shadow-DOM-aware walk (_isNodeInsideElement) instead of contains/shadowRoot.contains - Remove own-fallback movement info path from ToolbarItem; items without _itemNavigation or getToolbarMovementInfo are now treated as single tab stops - Drop dead WeakMap tab-index restoration machinery (no longer needed now that overflow items manage their own tab order)
GitHub Actions
Inactive
GitHub Actions
Inactive
plamenivanov91
changed the title
…ling when up/down keys are pressed
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
👋 Heads-up: dev close is in effectThanks for the contribution! This repository is currently in dev close ahead of release This PR appears to introduce public-API changes (detected by diffing the Custom Elements Manifest against the latest published version on npm):
Could you please hold off on merging into If this change must ship in the current release, please request a review from one or two members of @UI5/ui5-team-webc so the team can sign off explicitly.
Posted automatically by the Dev Close Notice workflow. |
GitHub Actions
Inactive
…avState interface
Replace the index-based ToolbarMovementInfo interface with a boolean
ToolbarArrowNavState { atLeftEnd, atRightEnd }. Items now report
whether they can absorb a keypress rather than exposing their internal
item count and cursor position.
- ToolbarItemBase/Item: remove ToolbarMovementInfo, ToolbarMovementEnabler,
moveWithinToolbarItem. Add getArrowNavState() returning
{ atLeftEnd, atRightEnd } (delegates to single child if present,
falls back to multi-child position check). Remove dead no-op branch
in focusForToolbarNavigation().
- Input/TextArea: getToolbarMovementInfo ? getArrowNavState, returning
{ atLeftEnd: caret===0, atRightEnd: caret>=len }.
- Toolbar: _applyRovingTabIndex sets all items to tabIndex=0 (Tab exits
naturally). _setCurrentItem only tracks _lastFocusedItem, no tabIndex
manipulation. Arrow nav guards via getArrowNavState(). Home/End also
guarded so caret-aware items (Input, TextArea) can handle them
internally. _moveToNext/_moveToPrev clamped at boundaries, no wrap.
- Tests: add no-wrap boundary, Tab navigation, Input caret-aware nav,
and Home key non-interception tests. Fix ToolbarSelect tests using
incorrect "click" event name and jQuery .get(0) anti-pattern.
GitHub Actions
Inactive
GitHub Actions
Inactive
…ocus model Addresses the three review comments on the WAI-ARIA toolbar navigation and applies follow-up hardening, without changing the established arrow/Tab/Home/End behaviour. Review comments - Remove the roving tabindex layer entirely (arrow nav uses composedPath / getActiveElement / _lastFocusedItem + .focus(), never tabindex; the forced tabindex also double-added a tab stop on composite items). Drops _applyRovingTabIndex, setToolbarForcedTabIndex, getToolbarFocusTarget, the forcedTabIndex property and its reset in addItemsAdditionalProperties. - isInteractive now checks whether the resolved focus target is actually focusable, so text-only content (ui5-text, ui5-title) no longer counts as a nav stop or toward role="toolbar". Resolves through getFocusDomRef so focus-delegating hosts (ui5-date-picker) stay navigable, and keeps href-less anchors that opt in via an explicit tabindex (ui5-link as a button, e.g. the Breadcrumbs overflow arrow) so a self-overflowed Breadcrumbs item is not skipped. - Remove dead code: the unused ToolbarArrowNavigation interface, the write-only _lastFocusedNavigationTarget, and the dead `?? true` in _getNavigableItems. Correctness hardening - Input/TextArea getArrowNavState: a non-collapsed selection is no longer a navigation boundary; Left/Right collapse the selection natively. - Home/End inside a multi-child group jump to the first/last child instead of being no-ops. - Reconcile _lastFocusedItem after reflow so Tab re-entry and arrow/Home/End don't silently restart from the first item. Lifecycle / cleanup - Attach focusin/keydown/close-overflow listeners once in onEnterDOM/onExitDOM instead of per render; cache composedPath and resolve overflowButtonDOM once. - Extract _resolveChildFocusTarget(); remove `as unknown as` double-casts, the `(this as any)` access, and the non-null assertion in the Tab branch. Tests (Toolbar.cy.tsx) - Text-only items non-interactive; focus-delegating hosts interactive; self-overflowed Breadcrumbs stays navigable. - Input range-selection stays in the input at a boundary; TextArea caret-aware navigation; RTL arrow direction; Home/End within a multi-child group. - ToolbarSelect programmatic-selection test stabilised against light-DOM option / rendered value.
GitHub Actions
Inactive
MapTo0
left a comment
MapTo0
left a comment
There was a problem hiding this comment.
Honestly, I do not understand the TA use case. Someone places a TA in the toolbar?
Implementation looks a bit like a magic. Can we implement an interface in the components that might be placed within a toolbar? something like IToolbarCompatible. Tomorrow if someone see this method, he/she would not know why its there at all.
Furthermore, considering all native inputs should be tackled you can also check other components: ComboBox, MultiComboBox, FileUploader (not sure), SearchField.
What about non ui5-webcomponents elements placed within a toolbar? Can users also mix components?
Hello there, We've discussed this implementation approach on the architecture round and there is nothing magical about it. We have three mechanisms used to decide whether or not we are on the first/last index inside a complex control nested in the ToolbarItem and they are all explained in the commit message.
The adaptation of the rest of controls you mentioned is not a Toolbar responsibility. The ones we altered are for demonstration purposes. Also they cover most of our sample code. For non ui5-webcomponents (like groups) placed within a ToolbarItem we have different mechanism as you can see in the code and in the commit message. Regards, |
4 participants
Replace Tab-through-every-item with a proper arrow-key navigation pattern
per the WAI-ARIA toolbar pattern.
Keyboard handling
[Left]/[Right]— navigate between toolbar items (RTL-aware); absorbed at the first/last item (no wrap)[Home]/[End]— jump to first/last item, unless the focused item manages its own internal navigation (e.g. Input caret, Breadcrumbs), in which case the item handles the key[Tab]/[Shift+Tab]— exit the toolbar via the browser's natural tab order; no roving tabindex, items keep their own natural tabindexfocusinhandling are suppressed while the popover is open. On open, the first interactive overflow item receives focus.Focus tracking
_lastFocusedItemtracks the current navigation target for re-entry (e.g. Shift+Tab back into the toolbar). It never mutates tabindex and is reconciled after reflow — a tracked item that moves to overflow or is removed no longer causes navigation to restart from the first item. When the overflow button disappears (items move back into the toolbar), focus moves to the last navigable item viafocusForToolbarNavigation.Interactivity detection
An item counts as interactive (and toward
role="toolbar") only when its resolved focus target is actually focusable:ui5-text,ui5-title) is skippedui5-date-picker) are resolved throughgetFocusDomRefand remain navigablehref-less anchors that opt in via an explicittabindex(e.g.ui5-linkrendered as a button, used by the Breadcrumbs overflow arrow) stay navigableToolbarItemBase — hook API for complex items
New overridable members (no-ops in base):
getArrowNavState()— reports{ atLeftEnd, atRightEnd }so the toolbar knows when a key press is at an internal boundary and should be handled by the item rather than moving to the next itemfocusForToolbarNavigation(isForward)— direction-aware focus entry pointToolbarItem — three navigation paths
_itemNavigationintrospection — components that useItemNavigationinternally (e.g. Breadcrumbs, SegmentedButton) are detected via duck-typing;currentIndex/itemCountderive the boundary stategetArrowNavStateinterface — explicit opt-in for components that expose caret/selection position as boundary info (e.g. Input, TextArea)ToolbarItems with more than one slotted child (radio/checkbox groups) are treated as a navigable group; Left/Right and Home/End move within the group, exiting only at its boundaryInput / TextArea — caret-aware boundary
Both implement
getArrowNavState()reportingatLeftEnd/atRightEndfrom the caret position, and only when the selection is collapsed — a non-collapsed selection is not treated as a boundary, so Left/Right collapse it natively instead of exiting to the next toolbar item.Accessibility docs
ui5-toolbaraccessibleName/accessibleNameRefTests (
Toolbar.cy.tsx,ToolbarSelect.cy.tsx)JIRA: BGSOFUIPIRIN-7018
Fixes: #12945